-
Notifications
You must be signed in to change notification settings - Fork 361
[PHP.wasm] Major overhaul of URL rewriting and setting $_SERVER variables #2864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
|
@tusharsnx btw the URL starting with /wp-content/ will not work anyway since every Playground lives at a subpath similar to /scope:my-site/. You'll need to use some WordPress helper to generate a site-relative URL, which will also solve the issue you are experiencing. I wonder if that makes this PR unnecessary, since we always expect that scope segment to come first 🤔 |
|
@adamziel I'm not sure if that's also true for Playground CLI. I can see that URLs are unscoped when using The full command I'm using to start Playground server: wp-playground-cli server --auto-mount --mount-before-install ./wordpress:/wordpressThe
This is an interesting discussion that happened in that thread. @bgrgicak Could you take a look here? |
|
Aha, I forgot these are also used in Playground CLI. Thank you for clarifying! Then yes, we need to address this. I actually started questioning whether we need this default rule at all. I think it's only needed on multisites? |
|
This turned out to be such a rabbit hole. URL rewrites never resolved the final URL correctly. In addition, the request handler never set the |
…nst real outcomes from other servers
| * If /var/www/file.php/index.php does not exist, but /var/www/file.php does, | ||
| * use /var/www/file.php. This is also what Apache and PHP Dev Server do. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL. Good catch!
| } | ||
| this.#setServerGlobalEntry( | ||
| 'PHP_SELF', | ||
| request.relativeUri || '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why || ''? When would an empty string make sense for PHP_SELF?
| * Request path following the domain:port part. | ||
| * Request path following the domain:port part – | ||
| * after any URL rewriting rules (e.g. apache .htaccess) | ||
| * have been applied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really appreciate all the little comments that are being deepened in this PR.

The Problem
The PHP request handler wasn't correctly applying URL rewriting and setting the correct
$_SERVERvariables. Apache distinguishes between unrewritten and rewritten URLs when setting$_SERVER['PHP_SELF'], $_SERVER['REQUEST_URI']` and other. Before this PR, Playground wasn't doing that.PHP Documentation on
$_SERVERhas some pointers, but they're far from exhaustive. I went ahead and started actual Apache and PHP Dev Server instances and noted their behavior in different scenarios, such as:I've found a lot of nuanced behaviors that I've described in details in the relevant contextual block comment.
Motivation for the change, related issues
As reported in #2855, the default Playground rewrite rule malforms this URL:
It transforms it to just:
At first I thought it's a small issue with the rewrite rule, and then I've discovered a rabbit hole of nuance.
Implementation
PHPRequestHandler.prepare$_SERVER()method that produces the right $_SERVER values given a specific request URL and its rewrite. It correctly populatesREQUEST_URI,PHP_SELF,SCRIPT_NAME,PATH_INFO, andQUERY_STRING. This partially overrides whatphp_wasm.cdo. I'd like to consolidate all that logic in a follow-up PR./file.php/index.phpwhere the full path doesn't exist but/file.phpdoes. This matches Apache and PHP Dev Server behavior.PHPRequestHandler.#applyRewriteRules()method that applies the URL rewrite and merges the newly generated query parameters with the ones from the the original URL.Test Coverage
Added test suites covering the same scenarios I've tested with Apache and PHP dev server. We send a request to a specific URL given some server-side rewrites or no rewrites at all, and then we confirm Playground produces
$_SERVERvalues consistent with those servers.